-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add ability to pass proxyOptions #459
Conversation
Please consider merging this PRwe really need to pass options to |
We ended up forking the project to add additional error handling to the proxy. Because in the current implementation if proxy throws an error the server would just crash. https://github.com/dcos-labs/http-server |
utc = argv.U || argv.utc, | ||
logger; | ||
|
||
var proxyOptionsBooleanProps = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the underlying library handle it's own validation, and we just surface any errors? I'd like to avoid duplicating other people's requirements (since they're likely to change over time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can pass stuff through without this boolean checking, so long as we handle errors from http-proxy in a user-friendly way.
@nLight would you be interested in submitting this as a PR? johntron@5aa9afd Did you add anything beyond that? |
@BigBlueHat you mean submitting the error handling to this repo? I can of course, though that still won't be sufficient for us without the secure flag. Also as you can see from the commit the logging solution is less then ideal. |
I'll open a PR and let's get the conversation started 👍 |
Can we please get this one merged and published? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this idea, and it would expose a ton of already-existing but hidden functionality! I'd like to drive for this to be in the next major release, if possible. There are some merge conflicts to address, though if you're too busy or no longer want to contribute here, I'd be willing to take over and make some of these changes before merging. Let me know though, I don't want to steal it from you if you want to work on it!
utc = argv.U || argv.utc, | ||
logger; | ||
|
||
var proxyOptionsBooleanProps = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can pass stuff through without this boolean checking, so long as we handle errors from http-proxy in a user-friendly way.
README.md
Outdated
@@ -53,6 +53,8 @@ This will install `http-server` globally so that it may be run from the command | |||
|
|||
`-P` or `--proxy` Proxies all requests which can't be resolved locally to the given url. e.g.: -P http://someurl.com | |||
|
|||
`--proxyOptions` Pass proxy [options](https://github.com/nodejitsu/node-http-proxy#options) using nested dotted objects. e.g.: --proxyOptions.secure false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that there are/have been several issues and pull requests that will be solved by this, I think it's worth adding more to this, especially .secure false
, which solves errors around self-signed certs (#214)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looking at the requested changes it doesn't look like anything big. Any way I can help with this? |
This comment has been minimized.
This comment has been minimized.
Closing in favor of #688 |
http-proxy library that is used in http-server support options.
This PR adds abilities to pass proxy options via cli or programmatically (helps in cases such like https://github.com/divhide/node-grunt-http-server)
Fixes #214, fixes #246